fix(compilers/openapi): preserve the bound a co-declaration loses - #346
Open
OmarAlJarrah wants to merge 1 commit into
Open
fix(compilers/openapi): preserve the bound a co-declaration loses#346OmarAlJarrah wants to merge 1 commit into
OmarAlJarrah wants to merge 1 commit into
Conversation
This was referenced Aug 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Under the 2020-12 dialect a side may declare both of its bound keywords —
minimumalongsideexclusiveMinimum,maximumalongsideexclusiveMaximum. They are conjunctive, andir.Constraintsholds one bound plus one exclusivity flag per side, so only one of them can reach a field. Which one that is was settled in #274: the tighter, named in a diagnostic together with the keyword it displaced.The displaced keyword went nowhere else.
{minimum: 10, exclusiveMinimum: 0}and{minimum: 10}compiled to byte-identical documents, and the only record that the source spelled the bound twice lived in a diagnostic message — a channel a consumer may filter by severity, and not part of what a downstream stage reads as data. A consumer reconstructing the source, or diffing two revisions of it, could not see the difference. That is the lossless-by-default invariant.The keyword that reaches no field is now kept verbatim under
UnmodeledwithReasonDegradedLoweringand provenance at the keyword's own pointer, on whichever carrier the reading position owns. All three carriers are covered: a model property, a parameter, and the alias node a component's body reduces to.Mechanically,
annotation.Constraintsgains the pointer and source index it needs to locate an entry — the same two argumentsannotation.Readalready takes — and returns the entry beside the constraints, which is what its three callers route onto their own carriers. The keyword is written back from the literal already read rather than by re-reading its raw node: both render the same JSON, since a numeric scalar converts through the samevalue.NumericLiteralthe bound came from, but only the first cannot fail, and a preservation that can fail needs a diagnostic saying which happened (#144). The diagnostic now says the keyword is kept verbatim, which it can state unconditionally for that reason.One nearby gap is deliberately left alone and filed as #343: the alias hoisted beside an unmodeled
oneOf/anyOfis the one alias hoist that reads no constraints at all, so a position that co-declares a union with structural keywords loses its bounds outright. That predates this change, is a different mechanism, and is noted at the call site.Test plan
TestReconcileBound_KeepsTheTighterOfTwoCoDeclaredBoundsnow asserts the entry the reconciliation leaves over — key, reason, exact literal, and pointer — over both sides and both directions of "which keyword wins", plus the ties and the magnitudes no rational holds.TestCoDeclaredBound_KeptOnTheCarrierThatReadIt(schema) andTestParams_CoDeclaredBoundKeptOnTheParameter(operation) drive whole specs through the compiler and assert the entry on each of the three carriers, in both directions; their negative twins assert that a side writing one keyword keeps nothing, so a bound never gets two homes.testdata/conformance/openapi/constraints.yamlgains a component whose body reduces to a shared primitive, so the corpus witnesses the alias-node carrier as well as the property one;assertCoDeclaredBoundKeptchecks all three entries. The regenerated golden adds only the preserved entries, the new component, and the reworded diagnostics.exclusiveMinimumfrom the new fixture reddensTestConformance/constraints.gofmt,go vet,golangci-lint,go build, and the coverage gate at exactly 100%.Closes #286